reloader should survive SyntaxError #994
Conversation
|
@benoitc plz review |
|
Couldn't this be handled in https://github.com/benoitc/gunicorn/blob/master/gunicorn/workers/base.py#L89 or even in the Reloader class? |
| ("Content-Type", "text/plain"), | ||
| ("Content-Length", str(len(msg))) | ||
| ]) | ||
| return iter([msg]) |
There was a problem hiding this comment.
Why do you need to add iter() here? If I remember correctly, [msg] is enough in the WSGI spec.
There was a problem hiding this comment.
oh I copied it from the example on http://gunicorn.org/ , and you're right, [msg] is enough
There was a problem hiding this comment.
@berkerpeksag I've removed the iter() here
|
@berkerpeksag I don't think this can be handled in |
|
This looks pretty good to me. |
|
@tilgovi what else should I do before this can be merged? |
|
I think it seems fine, I just want to give others a chance to look at it. |
|
The patch works for me. But I would do the following changes:
Thoughts? |
|
Perhaps it would be good to make the try-except logic a method. This way, people can customize make_fail_app etc. |
| try: | ||
| self.wsgi = self.app.wsgi() | ||
| except SyntaxError as e: | ||
| if not self.reloader: |
There was a problem hiding this comment.
I'd use self.cfg.reload
There was a problem hiding this comment.
+1
On Mon, Mar 16, 2015, 16:26 Berker Peksag notifications@github.com wrote:
In gunicorn/workers/base.py
#994 (comment):@@ -116,7 +117,19 @@ def changed(fname):
self.init_signals()
self.wsgi = self.app.wsgi()try:self.wsgi = self.app.wsgi()except SyntaxError as e:if not self.reloader:I'd use self.cfg.reload
—
Reply to this email directly or view it on GitHub
https://github.com/benoitc/gunicorn/pull/994/files#r26533913.
|
@wong2 no only |
|
I've moved the try-catch codes to a new method |
|
Looks great. |
reloader should survive SyntaxError
|
Thanks @wong2 :) |
|
🍺 |
This implements the feature requested in issue #964.
When there is
SyntaxErrorraised while importing the app, provides a fallback app to display those errors instead of completely exit gunicorn.